Enhance MCP authentication handler to correctly handle absolute URIs for resource metadata endpoint #937
Conversation
…for resource metadata endpoint
| .AddMcp(options => | ||
| { | ||
| // Set an absolute URI with a path component after the host | ||
| options.ResourceMetadataUri = new Uri($"{McpServerUrl}{fullMetadataPath}"); |
There was a problem hiding this comment.
Do we have a test for a custom relative path? If not, can we add that too?
| else | ||
| { | ||
| // For absolute URIs, extract the path component | ||
| expectedMetadataPath = Options.ResourceMetadataUri.AbsolutePath; |
There was a problem hiding this comment.
It seems wrong to throw away the host name when it's been explicitly configured. If the app developer configures AllowedHosts to something other than *, they are already protected against DNS rebinding, but not everyone does that.
But if the developer chooses a specific host name for the ResourceMetadataUri, it seems that we should require it. If the developer doesn't want to require a specific host name for the resource metadata data document, they can use a relative URI.
There was a problem hiding this comment.
I've learned via chat that the motivating reason for using an absolute ResourceMetadataUri was to fix the scenario where GetBaseUrl() => $"{Request.Scheme}://{Request.Host}{Request.PathBase}" gave inaccurate results due to a gateway in front of an app without properly configured forwarded headers. The result was that context.Request.Host contained an internal hostname rather than the publicly visible one specified in ResourceMetadataUri. Basically, this is exactly the scenario I suggested we reject for having the "wrong" host name.
For this scenario, I think using app.UseForwardedHeaders() and a relative ResourceMetadataUri is the best approach. Configuring forwarded headers could help fix other scenarios involving absolute link generation too.
Given that an absolute ResourceMetadataUri has never worked, I wonder if the best bet for now is simply to reject absolute URIs by throwing an ArgumentException in the setter. If people really do need to have complete control over the absolute URI generated for the Www-Authentication header, that could probably be a new configuration and/or McpAuthenticationEvents that gets run in HandleChallengeAsync.
2 participants
Enhance MCP authentication handler to correctly handle absolute URIs for resource metadata endpoint
Motivation and Context
This pull request refines how the resource metadata endpoint path is determined in the authentication handler and adds a comprehensive test to verify correct behavior when using absolute URIs with path components. The main focus is to ensure that requests to the metadata endpoint are routed and authenticated properly, especially when the endpoint is configured with an absolute URI that includes a path.
Authentication handler improvements:
McpAuthenticationHandler.csto correctly extract the path component fromResourceMetadataUriwhen it is an absolute URI, ensuring accurate endpoint matching.How Has This Been Tested?
ResourceMetadataEndpoint_ResolvesCorrectly_WithAbsoluteUriIncludingPathComponentinAuthTests.csto verify:WWW-Authenticateheader includes the full absolute URI with path component.System.Net.Http.Jsonin the test file to support JSON deserialization in the new test.Breaking Changes
None, existing PRM URIs will not be affected.
Types of changes
Checklist